GameFile: Support HBC-style XML metadata #8313
Conversation
|
|
||
| // Elements that we aren't using: | ||
| // version (can be written in any format) | ||
| // release_date (YYYYmmddHHMMSS format) |
There was a problem hiding this comment.
We do append the revision to the game name (altho this is a u16 and not just any plain string); and there would be the apploader date field (which is a string)...so we potentially could use those two.
There was a problem hiding this comment.
I would really not like to change the revision from u16 to std::string. I could do the apploader date, but an apploader date isn't really the same as a release date, and any custom data doesn't show up in the properties dialog anyway the way things are now (which is the only place where you can see the apploader date). Maybe we could rework this if we add a properties dialog for DOL/ELF files at some point.
Source/Core/UICommon/GameFile.h
Outdated
| const std::string& GetName(bool long_name = true) const; | ||
| const std::string& GetMaker(bool long_maker = true) const; | ||
| const std::string& GetName(bool allow_custom_name, bool long_name = true) const; | ||
| const std::string& GetMaker(bool allow_custom_maker, bool long_maker = true) const; |
There was a problem hiding this comment.
Those bools look iffy, especially since one of them is defaulted while the other is not. Maybe we should consider an enum instead; possibly even one that combines both as one (like GetName(Long | AllowCustom) or GetName(Short) etc.)
Thoughts?
There was a problem hiding this comment.
I could do enums, but we should avoid "combined" enums due to https://bugs.dolphin-emu.org/issues/11692.
There was a problem hiding this comment.
Does this also apply to enum class? I was just omitting stuff for brevity; I didn't necessarily mean for this to use an unqualified enum.
There was a problem hiding this comment.
With enum class I believe you get a compile error if you try to use | without casting, but I haven't tested it.
There was a problem hiding this comment.
Apparently you do get an error in that case, but you're allowed to declare your own | operator if you want it (according to this article). That's probably excessive for something like this, though...
There was a problem hiding this comment.
We actually use this method in URDE with great success, we have a macro in our IO library athena which defines all of the necessary operators:
https://gitlab.axiodl.com/libAthena/athena/blob/master/include/athena/Global.hpp#L88-120
Then it's used like this:
https://gitlab.axiodl.com/AxioDL/hecl/blob/master/include/hecl/CVar.hpp#L27
There was a problem hiding this comment.
Both parameters are now using one enum class. (I thought merging them into one would be a good idea, both to require less typing for each function call and because the "custom and short" combination doesn't make sense since custom names don't have a limited length.)
JosJuice
force-pushed
the
gamelist-xml
branch
2 times, most recently
from
fe4b4c1
to
32a014b
Compare
This feature was originally exclusive to the previous iteration of DolphinQt (the one that was the reason for the current iteration being named DolphinQt2 initially). https://bugs.dolphin-emu.org/issues/8949
JosJuice
force-pushed
the
gamelist-xml
branch
from
32a014b
to
d8958fb
Compare
| { | ||
| LongAndPossiblyCustom, | ||
| LongAndNotCustom, | ||
| ShortAndNotCustom, |
There was a problem hiding this comment.
This will be the nittiest of nits, but...thoughts on using LongButNotCustom, ShortButNotCustom? :)
But then again, should it be LongOrPossiblyCustom for the first one?
You should probably ignore this comment for the sake of all the sheds we built so far.
There was a problem hiding this comment.
Using But there doesn't make too much sense to me, since there's no logical opposition between Long and Custom in LongButNotCustom. Also, it feels slightly awkward to make it inconsistent by using But for the last two ones but not the first one.
This feature was originally exclusive to the previous iteration of DolphinQt (the one that was the reason for the current iteration being named DolphinQt2 initially).
https://bugs.dolphin-emu.org/issues/8949